Skip to content

fix(bedrock-converse): parse streaming tool_kwargs from string to dict#21580

Open
ayushh0110 wants to merge 1 commit intorun-llama:mainfrom
ayushh0110:fix/bedrock-converse-streaming-tool-kwargs-parse
Open

fix(bedrock-converse): parse streaming tool_kwargs from string to dict#21580
ayushh0110 wants to merge 1 commit intorun-llama:mainfrom
ayushh0110:fix/bedrock-converse-streaming-tool-kwargs-parse

Conversation

@ayushh0110
Copy link
Copy Markdown

Description

Fixes #21579

Bedrock ConverseStream delivers tool use input as partial JSON string chunks via ToolUseBlockDelta.input. The streaming code (stream_chat / astream_chat) correctly concatenates these chunks into a complete JSON string, but passes the raw string directly to ToolCallBlock.tool_kwargs instead of parsing it into a dict.

This breaks cross-provider workflows where chat history from a Bedrock streaming session is replayed through another provider (e.g., Google GenAI), which expects tool_kwargs to be a dict:

pydantic_core.ValidationError: 1 validation error for FunctionCall
args
  Input should be a valid dictionary [type=dict_type, input_value='{"document_id": "..."}', input_type=str]

The non-streaming path works fine because boto3 automatically deserializes ToolUseBlock.input to a Python dict.

Changes

  • Added _parse_tool_input() helper function that safely parses JSON strings to dicts (with fallback to {} for incomplete/malformed JSON during intermediate streaming yields)
  • Applied the helper at all 6 ToolCallBlock construction sites in stream_chat (3 sites) and astream_chat (3 sites)
  • Added 2 new unit tests (test_stream_chat_tool_kwargs_parsed_as_dict and test_astream_chat_tool_kwargs_parsed_as_dict) that mock the full Bedrock streaming tool use event sequence and assert tool_kwargs is a dict

Why this approach

The @dosu analysis suggested parsing at the contentBlockStop event. However, the current code has no contentBlockStop handler, and intermediate contentBlockDelta yields would still emit raw strings before that event fires.

Instead, wrapping at each construction site:

  • Normalizes every yield (intermediate and final)
  • Requires no new event handlers or structural changes
  • Matches the existing defensive pattern in get_tool_calls_from_response() (which already does isinstance(tool_call.tool_kwargs, str)parse_partial_json)
  • Minimal diff — easy to review

Test results

83 passed, 19 skipped (AWS integration tests)

All existing tests pass with zero regressions.

Bedrock ConverseStream delivers tool use input as string chunks. The
streaming code correctly concatenates these into a complete JSON string,
but passes the raw string directly to ToolCallBlock.tool_kwargs instead
of parsing it into a dict.

This breaks cross-provider workflows where chat history from a Bedrock
streaming session is replayed through another provider (e.g. Google
GenAI) that expects tool_kwargs to be a dict.

Add _parse_tool_input() helper that safely parses JSON strings to dicts
and apply it at all 6 ToolCallBlock construction sites in stream_chat
and astream_chat.

Fixes run-llama#21579
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label May 6, 2026
@dosubot
Copy link
Copy Markdown

dosubot Bot commented May 6, 2026

This looks like a solid fix! The approach is clean, correct, and well-tested. A few observations:

What works well:

  • The _parse_tool_input() helper is simple and focused — it normalizes tool input consistently across all 6 streaming construction sites [1]
  • Using json.loads() (rather than parse_partial_json()) is appropriate here since the string is fully accumulated by the time it's parsed
  • The empty dict {} fallback on parse failures matches the existing defensive pattern in get_tool_calls_from_response() [2]
  • Comprehensive test coverage for both sync and async paths

On the approach difference from the Dosu suggestion: You noted you went with wrapping at each construction site instead of a single contentBlockStop handler. Your reasoning is sound — since intermediate contentBlockDelta yields already construct ToolCallBlock objects, normalizing at every site ensures all emitted blocks have dict tool_kwargs, not just the final one. This is a pragmatic choice that avoids introducing new event handling logic.

One minor note: intermediate yields during streaming will have incomplete JSON strings that json.loads() will fail to parse, resulting in {} being yielded for tool_kwargs until the final chunk arrives. This is acceptable behavior (consumers shouldn't act on intermediate tool call data), but worth being aware of.

Overall this is a clean, minimal fix that directly addresses the cross-provider incompatibility reported in #21579 [3]. Nice work! 🎉

To reply, just mention @dosu.


Docs are dead. Just use Dosu.

Leave Feedback Ask Dosu about llama_index Share Dosu with your team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Bedrock Converse streaming produces string tool_kwargs in ToolCallBlock instead of dict

1 participant